Skip to content

Do not hide SPA page while JS is loading #1929

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

exyi
Copy link
Member

@exyi exyi commented Apr 26, 2025

Should improve "visual" loads times, and SEO.

Hiding the page is currently unnecessary
it was added back in 2015 (9c11159), because we used to rely on the URL path fragment (#/something), which could be different from the page which was currently being loaded.

@exyi exyi added this to the Version 5.0 milestone Apr 26, 2025
@tomasherceg
Copy link
Member

Seems like this change broke down several SPA-related tests.

exyi added 2 commits June 1, 2025 11:30
Should improve "visual" loads times, and SEO.

Hiding the page is currently unnecessary
it was added back in 2015 (9c11159), because we used
to rely on the URL path fragment (`#/something`), which
could be different from the page which was currently being loaded.
...but, we can replace it with `if: true`, which does not have the unwanted effect of hiding
the content before everything loads.

The reson is that ko.bindingContext/updateContext is pureComputed and won't update
$data right after we replace the viewModel. We need someone to call `context[contextSubscribable]` to update it.
Any binding will do that, but only after it's initialized, so the initialization will fail on
"Property X was not defined".

The pureComputed was introduced in knockout/knockout@0ac7010.
It's probably not a knockout bug, as it's not possible to reproduce without our hacks with
applyBindings in restoreUpdatedControls. If we used a binding handler for this, everything
would be fine.
@exyi exyi force-pushed the spa-dont-hide-page-before-js-loads branch from 05ba373 to a7f4b56 Compare June 1, 2025 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants